Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix styles for hub pages and article page #9765

Merged
merged 39 commits into from
Jul 13, 2022
Merged

fix styles for hub pages and article page #9765

merged 39 commits into from
Jul 13, 2022

Conversation

marcochavezf
Copy link
Contributor

@marcochavezf marcochavezf commented Jul 7, 2022 •
edited
Loading

cc @roryabraham @michelle-thompson

Details

This PR includes mainly style changes for the Help site to mimic the mockups design from the design doc.

Some other minor changes included in this PR:

  • Update outdated links from comments in the README.md file
  • Rename files from the _includes folder (changed -navigation-tree suffix to -lhn)
  • Add functionality to toggle the menu in small views (mobile web)

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/216742

Tests

  1. (you need ruby and bundler installed to do this, more info here)

  2. Run:

    cd docs
bundle install
bundle exec jekyll serve

  3. Make sure the site's styles load and that all the colors look correct.

Additionally add more content like Stage titles, Groups of cards and card buttons to one of the hub pages (i.e. request-money and Stage title anchors and Article links to the respective LHN section (i.e. request-money-lhn).

Screenshots

Hub page

Desktop (updated):
localhost_4000_hubs_request-money_

Mobile (updated):

Article page

Desktop:
localhost_4000_articles_request-money_SmartScan

Mobile:

@marcochavezf marcochavezf self-assigned this Jul 7, 2022
@marcochavezf marcochavezf changed the title [WIP] fix hub pages 2 [WIP] fix hub pages Jul 7, 2022
@marcochavezf marcochavezf changed the title [WIP] fix hub pages fix styles for hub pages and article page Jul 9, 2022
@marcochavezf marcochavezf marked this pull request as ready for review July 9, 2022 22:24
@marcochavezf marcochavezf requested a review from a team as a code owner July 9, 2022 22:24
@melvin-bot melvin-bot bot requested review from MariaHCD and removed request for a team July 9, 2022 22:24
MariaHCD
MariaHCD reviewed Jul 11, 2022
View reviewed changes
Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just a few comments.

docs/hubs/request-money.html Show resolved Hide resolved
docs/_includes/home-lhn.html Outdated Show resolved Hide resolved
docs/_layouts/default.html Show resolved Hide resolved
@michelle-thompson
Copy link
Contributor

image

The icon is the wrong color - it should be the same as the icons in the LHN. Additionally, the border is too dark - let's change it to #ECECEC

@marcochavezf
Copy link
Contributor Author

image

The icon is the wrong color - it should be the same as the icons in the LHN. Additionally, the border is too dark - let's change it to #ECECEC

Good catch @michelle-thompson! I updated the styles for the card in the hub pages and also updated the screenshots in the PR description.

@MariaHCD I addressed the requested changes, thanks for the review! 🙇🏽 The PR is ready to be reviewed again.

@marcochavezf marcochavezf requested a review from MariaHCD July 11, 2022 20:10
@michelle-thompson
Copy link
Contributor

One more comment, I noticed the hyperlinks are medium weight. Links should be regular weight (unless the body text is bolded, in which case it should follow the body text)
image

@marcochavezf
Copy link
Contributor Author

marcochavezf commented Jul 12, 2022 •
edited
Loading

One more comment, I noticed the hyperlinks are medium weight. Links should be regular weight (unless the body text is bolded, in which case it should follow the body text) image

@michelle-thompson Ah yes, it was a bolded hyperlink, I think I accidentally added the bold property in the article .md content. This how the link looks like without the bold property:

localhost_4000_articles_request-money_SmartScan (1)

@michelle-thompson
Copy link
Contributor

Looks great, thank you!

@marcochavezf marcochavezf mentioned this pull request Jul 12, 2022
Merged
@ctkochan22
Copy link

Reviewing the other PR: #9869 (comment)

It looks like we are good right? It seems @michelle-thompson gave it the thumbs up.

@ctkochan22 ctkochan22 merged commit 0c5cf60 into main Jul 13, 2022
@ctkochan22 ctkochan22 deleted the marco-fixHubPages-2 branch July 13, 2022 15:31
@OSBotify OSBotify mentioned this pull request Jul 13, 2022
Closed
95 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @chiragsalian in version: 1.1.84-13 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

allroundexperts
allroundexperts reviewed Sep 17, 2023
View reviewed changes
docs/_sass/_main.scss
Comment on lines +200 to +202
.header-button {
position: absolute;
display: none;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused #25646 down the road.

@allroundexperts allroundexperts mentioned this pull request Sep 17, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allroundexperts allroundexperts allroundexperts left review comments

@MariaHCD MariaHCD MariaHCD approved these changes

@ctkochan22 ctkochan22 ctkochan22 approved these changes

@michelle-thompson michelle-thompson Awaiting requested review from michelle-thompson

Assignees

@marcochavezf marcochavezf

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@marcochavezf @michelle-thompson @ctkochan22 @OSBotify @MariaHCD @allroundexperts